[GEN][ZH] Annotate fallthrough between various switch case labels #1085
Conversation
xezon
added
Minor
xezon
force-pushed
the
xezon/staticanalysis/001
branch
from
812d386 to
d88e895
Compare
| case CHINA_FRIEND: | ||
| side.set("China"); | ||
| break; | ||
| case GLA_ENEMY: | ||
| isFriend = FALSE; | ||
| isFriend = FALSE; |
There was a problem hiding this comment.
Seems odd that it thinks there was a change here.
There was a problem hiding this comment.
Seems odd that it thinks there was a change here.
There's a tab after it in the original line.
| case SCORESCALEUPTRANSITION_2: | ||
| case SCORESCALEUPTRANSITION_3: |
There was a problem hiding this comment.
I am surprised it does not warn about these cases falling through.
| FALLTHROUGH; | ||
| case 2: | ||
| break; |
There was a problem hiding this comment.
State is not used outside of this function so this seems a bit redundant, case 2 could be removed and a break added at the end of case 1.
There was a problem hiding this comment.
You are probably right. I think the change as is is sufficient for now, so it is in line with the title of this change.
| FALLTHROUGH; | ||
|
|
||
| case FLASHTRANSITION_FADE_IN_2: | ||
| case FLASHTRANSITION_FADE_IN_3: |
There was a problem hiding this comment.
Same as the score scaleup, im surprised it does not complain here, unless it just optimises away empty cases?
There was a problem hiding this comment.
Yes it looks like the static analyzer does not care about the fallthrough annotation when there is no body behind the switch case label. It is a very common pattern and I think fallthrough is expected implicitly in this case, so requires no annotation to be clear.
This change annotates fallthroughs in GameEngine to suppress warnings and make the intent clear.